Conversation
Previously it was using 7d by default… even though the Metrics default is 1d 🤦♂️
In the graph largest at the bottom, the legend is largest at the top
|
WalkthroughChanges add aggregation-aware legend totals and null-aware gap handling across chart components: a new shared Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used📓 Path-based instructions (7)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
🧠 Learnings (13)📓 Common learnings📚 Learning: 2026-02-11T16:37:32.429ZApplied to files:
📚 Learning: 2025-12-08T15:19:56.823ZApplied to files:
📚 Learning: 2026-02-11T16:50:07.201ZApplied to files:
📚 Learning: 2026-02-03T18:27:40.429ZApplied to files:
📚 Learning: 2025-11-27T16:26:58.661ZApplied to files:
📚 Learning: 2026-02-10T16:18:48.654ZApplied to files:
📚 Learning: 2026-02-04T16:34:48.876ZApplied to files:
📚 Learning: 2025-09-03T14:35:52.384ZApplied to files:
📚 Learning: 2025-11-27T16:26:58.661ZApplied to files:
📚 Learning: 2026-01-28T16:57:47.620ZApplied to files:
📚 Learning: 2025-07-12T18:06:04.133ZApplied to files:
📚 Learning: 2026-02-06T19:53:38.843ZApplied to files:
🧬 Code graph analysis (4)apps/webapp/app/components/query/QueryEditor.tsx (1)
apps/webapp/app/components/primitives/charts/ChartRoot.tsx (2)
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx (5)
apps/webapp/app/components/code/QueryResultsChart.tsx (4)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (14)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/webapp/app/services/queryService.server.ts (1)
140-152:⚠️ Potential issue | 🟡 MinorUser-facing error message still says "organization" after scoping to
projectId.The concurrency key is now
projectId, but the error message on line 150 still reads "for this organization". This will confuse users hitting the limit.Proposed fix
acquireResult.reason === "key_limit" - ? `You've exceeded your query concurrency of ${orgLimit} for this organization. Please try again later.` + ? `You've exceeded your query concurrency of ${orgLimit} for this project. Please try again later.` : "We're experiencing a lot of queries at the moment. Please try again later.";Also consider renaming
orgLimit→projectLimitandcustomOrgConcurrencyLimit→customProjectConcurrencyLimitfor consistency with the new scoping.apps/webapp/app/components/code/QueryResultsChart.tsx (1)
628-630:⚠️ Potential issue | 🟡 MinorGrouped pivot uses
0for missing group values while gap-fill usesnull.On line 629, when a group has no values for a particular x-axis bucket, it defaults to
0:point[group] = values[group] ? aggregateValues(values[group], aggregation) : 0;This means these "missing group" entries will be counted as real data in avg/min/max legend calculations, unlike gap-filled slots which use
nulland are correctly skipped. For bar charts this is likely the intended behavior (an absent group at a time slot truly contributed 0), but it's worth confirming this is desired for min aggregation — a group that never appeared in a time bucket would pullminto 0.💡 Consider using `null` for missing groups (if min/max should skip them)
- point[group] = values[group] ? aggregateValues(values[group], aggregation) : 0; + point[group] = values[group] ? aggregateValues(values[group], aggregation) : null;apps/webapp/app/presenters/v3/BuiltInDashboards.server.ts (1)
2-2:⚠️ Potential issue | 🟡 MinorRemove unused
zodimport.
zfromzodis imported on line 2 but never used in this file.🧹 Proposed fix
-import { z } from "zod";
🤖 Fix all issues with AI agents
In `@apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx`:
- Around line 62-70: grandTotal currently computes an unweighted "average of
averages" when aggregation === "avg" by calling aggregateValues on per-series
averages (const grandTotal = useMemo(... aggregateValues(values, aggregation))),
which misweights series of different lengths; change the logic in the grandTotal
useMemo: if aggregation === "avg" compute a weighted average using per-series
counts (e.g., use totals[key] * counts[key] summed over dataKeys divided by sum
of counts) instead of aggregateValues(values, "avg"), otherwise keep the
existing behavior; ensure you source the per-series counts (e.g., seriesCounts
or length from the original series data) and reference totals, dataKeys,
aggregation, aggregateValues, and grandTotal when updating the code.
🧹 Nitpick comments (3)
apps/webapp/app/components/primitives/charts/aggregation.ts (1)
9-22:Math.min(...values)/Math.max(...values)can overflow the call stack on large arrays.Spreading into
Math.min/Math.maxis limited by the engine's max argument count (~65k–125k elements). Chart series are typically small, but if this utility is reused elsewhere, consider a reduce-based approach for safety.♻️ Reduce-based alternative
case "min": - return Math.min(...values); + return values.reduce((a, b) => (b < a ? b : a), values[0]); case "max": - return Math.max(...values); + return values.reduce((a, b) => (b > a ? b : a), values[0]);apps/webapp/app/components/primitives/charts/ChartRoot.tsx (1)
213-224:|| 0coercion masks legitimate zero values in the sum/count path.Line 218 uses
Number(item[seriesKey] || 0), which treats bothnulland actual0values the same way. While this works correctly for sum (null → 0, 0 → 0), it's inconsistent with the avg/min/max branches that explicitly skipnullvia== null. For clarity and consistency, consider aligning the null-skipping behavior.That said, for additive aggregation, adding 0 for null values produces the same result as skipping them, so this is not a functional bug.
♻️ Optional: align null handling with other branches
- acc[seriesKey] = (acc[seriesKey] || 0) + Number(item[seriesKey] || 0); + const rawVal = item[seriesKey]; + if (rawVal == null) continue; // skip gap-filled nulls (consistent with avg/min/max) + acc[seriesKey] = (acc[seriesKey] || 0) + Number(rawVal);apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx (1)
200-210: Reduced opacity for null/zero items — consider separating the two cases.Line 209 applies
opacity-50for bothnull(gap-filled, no data) and0(explicit zero value). A user might want to distinguish "no data exists" from "data exists but value is zero" — the former being truly empty and the latter being a meaningful data point. This is a minor UX consideration.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/components/code/QueryResultsChart.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/env.server.tsapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/services/queryService.server.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/components/code/QueryResultsChart.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/components/code/QueryResultsChart.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/components/code/QueryResultsChart.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/components/code/QueryResultsChart.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webappAccess environment variables via
envexport fromapps/webapp/app/env.server.ts, never useprocess.envdirectly
Files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/components/code/QueryResultsChart.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/app/env.server.tsapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/presenters/v3/BuiltInDashboards.server.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/charts/aggregation.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/presenters/v3/BuiltInDashboards.server.tsapps/webapp/app/components/code/QueryResultsChart.tsx
apps/webapp/app/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Separate testable services from configuration files; follow the pattern of
realtimeClient.server.ts(testable service) andrealtimeClientGlobal.server.ts(configuration) in the webapp
Files:
apps/webapp/app/services/queryService.server.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131
Timestamp: 2026-02-11T16:50:07.201Z
Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.
📚 Learning: 2025-11-14T16:03:06.917Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2681
File: apps/webapp/app/services/platform.v3.server.ts:258-302
Timestamp: 2025-11-14T16:03:06.917Z
Learning: In `apps/webapp/app/services/platform.v3.server.ts`, the `getDefaultEnvironmentConcurrencyLimit` function intentionally throws an error (rather than falling back to org.maximumConcurrencyLimit) when the billing client returns undefined plan limits. This fail-fast behavior prevents users from receiving more concurrency than their plan entitles them to. The org.maximumConcurrencyLimit fallback is only for self-hosted deployments where no billing client exists.
Applied to files:
apps/webapp/app/env.server.tsapps/webapp/app/services/queryService.server.tsapps/webapp/app/routes/resources.metric.tsx
📚 Learning: 2026-02-10T16:18:48.654Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 2980
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx:512-515
Timestamp: 2026-02-10T16:18:48.654Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx, environment.queueSizeLimit is a per-queue maximum that is configured at the environment level, not a shared limit across all queues. Each queue can have up to environment.queueSizeLimit items queued independently.
Applied to files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/resources.metric.tsx
📚 Learning: 2026-02-11T16:50:07.201Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131
Timestamp: 2026-02-11T16:50:07.201Z
Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.
Applied to files:
apps/webapp/app/env.server.tsapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/services/queryService.server.tsapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/presenters/v3/BuiltInDashboards.server.ts
📚 Learning: 2025-12-08T15:19:56.823Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2760
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx:278-281
Timestamp: 2025-12-08T15:19:56.823Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsx, the tableState search parameter uses intentional double-encoding: the parameter value contains a URL-encoded URLSearchParams string, so decodeURIComponent(value("tableState") ?? "") is required to fully decode it before parsing with new URLSearchParams(). This pattern allows bundling multiple filter/pagination params as a single search parameter.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsx
📚 Learning: 2026-02-03T18:27:40.429Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx:553-555
Timestamp: 2026-02-03T18:27:40.429Z
Learning: In apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables/route.tsx, the menu buttons (e.g., Edit with PencilSquareIcon) in the TableCellMenu are intentionally icon-only with no text labels as a compact UI pattern. This is a deliberate design choice for this route; preserve the icon-only behavior for consistency in this file.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2026-02-04T16:34:48.876Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/routes/vercel.connect.tsx:13-27
Timestamp: 2026-02-04T16:34:48.876Z
Learning: In apps/webapp/app/routes/vercel.connect.tsx, configurationId may be absent for "dashboard" flows but must be present for "marketplace" flows. Enforce this with a Zod superRefine and pass installationId to repository methods only when configurationId is defined (omit the field otherwise).
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsx
📚 Learning: 2025-09-03T14:35:52.384Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2464
File: apps/webapp/app/utils/pathBuilder.ts:144-146
Timestamp: 2025-09-03T14:35:52.384Z
Learning: In the trigger.dev codebase, organization slugs are safe for URL query parameters and don't require URL encoding, as confirmed by the maintainer in apps/webapp/app/utils/pathBuilder.ts.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/routes/resources.metric.tsx
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : When importing from `trigger.dev/core` in the webapp, use subpath exports from the package.json instead of importing from the root path
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsxapps/webapp/app/components/primitives/charts/ChartLine.tsxapps/webapp/app/routes/resources.metric.tsxapps/webapp/app/components/primitives/charts/ChartRoot.tsxapps/webapp/app/components/code/ChartConfigPanel.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsxapps/webapp/app/components/query/QueryEditor.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/code/QueryResultsChart.tsx
📚 Learning: 2026-01-12T17:18:09.451Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2870
File: apps/webapp/app/services/redisConcurrencyLimiter.server.ts:56-66
Timestamp: 2026-01-12T17:18:09.451Z
Learning: In `apps/webapp/app/services/redisConcurrencyLimiter.server.ts`, the query concurrency limiter will not be deployed with Redis Cluster mode, so multi-key operations (keyKey and globalKey in different hash slots) are acceptable and will function correctly in standalone Redis mode.
Applied to files:
apps/webapp/app/services/queryService.server.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Control concurrency using the `queue` property with `concurrencyLimit` option
Applied to files:
apps/webapp/app/routes/resources.metric.tsx
📚 Learning: 2026-01-08T15:57:09.323Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/otel-metrics.mdc:0-0
Timestamp: 2026-01-08T15:57:09.323Z
Learning: Reference the schedule engine (internal-packages/schedule-engine/src/engine/index.ts) as a good example of implementing low-cardinality metric attributes
Applied to files:
apps/webapp/app/routes/resources.metric.tsx
📚 Learning: 2026-01-28T16:57:47.620Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 2964
File: apps/webapp/app/components/AskAI.tsx:121-141
Timestamp: 2026-01-28T16:57:47.620Z
Learning: In the trigger.dev webapp codebase, the Button component (apps/webapp/app/components/primitives/Buttons) does not spread unknown props to the underlying <button> element—it only passes specific props (type, disabled, onClick, name, value, ref, form, autoFocus). When using Radix UI's TooltipTrigger with asChild, a span wrapper around the Button is necessary to receive Radix props (aria-describedby, onPointerEnter, onPointerLeave, data-state) while the Button handles its own behavior. Directly making the Button the child of TooltipTrigger with asChild will break accessibility and tooltip functionality.
Applied to files:
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx
📚 Learning: 2025-07-12T18:06:04.133Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 2264
File: apps/webapp/app/services/runsRepository.server.ts:172-174
Timestamp: 2025-07-12T18:06:04.133Z
Learning: In apps/webapp/app/services/runsRepository.server.ts, the in-memory status filtering after fetching runs from Prisma is intentionally used as a workaround for ClickHouse data delays. This approach is acceptable because the result set is limited to a maximum of 100 runs due to pagination, making the performance impact negligible.
Applied to files:
apps/webapp/app/presenters/v3/BuiltInDashboards.server.ts
📚 Learning: 2026-02-06T19:53:38.843Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: apps/webapp/app/presenters/v3/DeploymentListPresenter.server.ts:233-237
Timestamp: 2026-02-06T19:53:38.843Z
Learning: When constructing Vercel dashboard URLs from deployment IDs, always strip the dpl_ prefix from the ID. Implement this by transforming the ID with .replace(/^dpl_/, "") before concatenating into the URL: https://vercel.com/${teamSlug}/${projectName}/${cleanedDeploymentId}. Consider centralizing this logic in a small helper (e.g., getVercelDeploymentId(id) or a URL builder) and add tests to verify both prefixed and non-prefixed inputs.
Applied to files:
apps/webapp/app/presenters/v3/BuiltInDashboards.server.ts
🧬 Code graph analysis (5)
apps/webapp/app/components/primitives/charts/aggregation.ts (1)
apps/webapp/app/components/metrics/QueryWidget.tsx (1)
AggregationType(45-45)
apps/webapp/app/routes/resources.metric.tsx (1)
apps/webapp/app/env.server.ts (1)
env(1336-1336)
apps/webapp/app/components/primitives/charts/ChartRoot.tsx (2)
apps/webapp/app/components/metrics/QueryWidget.tsx (1)
AggregationType(45-45)apps/webapp/app/components/primitives/charts/ChartContext.tsx (1)
useChartContext(43-49)
apps/webapp/app/components/query/QueryEditor.tsx (1)
apps/webapp/app/components/code/ChartConfigPanel.tsx (1)
defaultChartConfig(17-27)
apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx (3)
apps/webapp/app/components/primitives/charts/ChartRoot.tsx (1)
useSeriesTotal(209-280)apps/webapp/app/components/primitives/charts/aggregation.ts (1)
aggregateValues(9-23)apps/webapp/app/components/primitives/Tooltip.tsx (1)
SimpleTooltip(141-141)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: sdk-compat / Deno Runtime
🔇 Additional comments (20)
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/route.tsx (2)
1-5: No issues found.Formatting-only change is fine.
90-97: No issues found.Whitespace-only change is fine.
apps/webapp/app/components/code/ChartConfigPanel.tsx (1)
150-154: LGTM — stable key approach prevents unnecessary re-runs.The
columnsKeystring is a sound proxy for the column structure, and the refs ensureconfig/onChangeare always fresh without being dependencies. The ESLint suppression is justified and well-documented.apps/webapp/app/components/primitives/charts/ChartLine.tsx (1)
189-221: LGTM — top margin and dot styling look correct.The added
top: 5margin prevents dot clipping, and the small dots (r: 1.5) provide subtle data-point visibility without overwhelming the chart.apps/webapp/app/routes/resources.metric.tsx (1)
15-15: LGTM — env-based concurrency limit properly replaces the hardcoded value.Uses the centralized
envexport as required by coding guidelines, and the server-only import is correctly scoped to theactionfunction.Also applies to: 111-111
apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.metrics.$dashboardKey/route.tsx (1)
229-229: LGTM — correct fallback todefaultPeriodinstead of null.This ensures widgets use the dashboard's configured default period (e.g.,
"1d") when no explicit period is selected, fixing the fallback behavior described in the PR objectives.apps/webapp/app/components/query/QueryEditor.tsx (1)
509-541: LGTM — improved chart config preservation logic.The approach of tracking only column names (not types), using a ref for the config, and resetting only when a configured column disappears is a solid improvement. It correctly preserves settings across re-runs of the same query while still resetting when the schema changes in a way that invalidates the config.
apps/webapp/app/env.server.ts (1)
1217-1218:⚠️ Potential issue | 🟡 MinorNaming says "ORG" but the concurrency key is now scoped to
projectId.In
queryService.server.ts, the concurrency limiter key was changed fromorganizationIdtoprojectId. This env var name (METRIC_WIDGET_DEFAULT_ORG_CONCURRENCY_LIMIT) suggests an org-level limit, but it's actually applied per-project. Consider renaming toMETRIC_WIDGET_DEFAULT_PROJECT_CONCURRENCY_LIMITto avoid operator confusion.⛔ Skipped due to learnings
Learnt from: matt-aitken Repo: triggerdotdev/trigger.dev PR: 3019 File: apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx:126-131 Timestamp: 2026-02-11T16:50:07.201Z Learning: In apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboards.$dashboardId.widgets.tsx, MetricsDashboard entities are intentionally scoped to the organization level, not the project level. The dashboard lookup should filter by organizationId only (not projectId), allowing dashboards to be accessed across projects within the same organization. The optional projectId field on MetricsDashboard serves other purposes and should not be used as an authorization constraint.Learnt from: matt-aitken Repo: triggerdotdev/trigger.dev PR: 2681 File: apps/webapp/app/services/platform.v3.server.ts:258-302 Timestamp: 2025-11-14T16:03:06.917Z Learning: In `apps/webapp/app/services/platform.v3.server.ts`, the `getDefaultEnvironmentConcurrencyLimit` function intentionally throws an error (rather than falling back to org.maximumConcurrencyLimit) when the billing client returns undefined plan limits. This fail-fast behavior prevents users from receiving more concurrency than their plan entitles them to. The org.maximumConcurrencyLimit fallback is only for self-hosted deployments where no billing client exists.Learnt from: CR Repo: triggerdotdev/trigger.dev PR: 0 File: .cursor/rules/otel-metrics.mdc:0-0 Timestamp: 2026-01-08T15:57:09.323Z Learning: Applies to **/*.ts : Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)apps/webapp/app/presenters/v3/BuiltInDashboards.server.ts (1)
32-32: LIMIT removal and query updates look correct.The LIMIT 100 removal across all dashboard queries aligns with the PR objective and is consistently applied. The queries retain their ORDER BY clauses where needed. The aggregation changes to
"avg"for percentage-based metrics (success %, version success %) make semantic sense — summing percentages would be meaningless.Also applies to: 38-38, 50-50, 57-57, 73-73, 79-79, 86-86, 102-102, 135-135, 141-141, 158-158, 209-209
apps/webapp/app/components/primitives/charts/ChartRoot.tsx (3)
3-3: Clean prop threading forlegendAggregation.The new
legendAggregationprop is properly threaded fromChartRoot→ChartRootInner→ChartLegendCompoundwith consistent typing using the type-only import. Looks good.Also applies to: 33-34, 79-79, 103-103, 120-120, 133-133, 175-175
245-261: Min aggregation:val < result[seriesKey]comparison whenresult[seriesKey]isundefined.On line 251, the condition
result[seriesKey] === undefined || val < result[seriesKey]is safe because the=== undefinedcheck short-circuits the comparison. The same pattern at line 269 for max is also correct. Implementation looks good.
189-201:useHasNoDatamemo deps look correct.The memo body uses
dataanddataKeys(via thekeyvariable fromdataKeys.every), and both are listed in the dependency array. ThedataKey(singular) from context is not used inside, so it's correctly omitted.apps/webapp/app/components/code/QueryResultsChart.tsx (4)
761-777: Series sorting by descending total sum looks good.The
sortedSeriescomputation correctly usesMath.abs(val)for magnitude-based ordering, guards withisFinite, and short-circuits when there's only one series. This ensures the largest series appears at the bottom of stacked charts and first in the legend.
246-258: Gap-filling withnullis the right approach for line charts and aggregation.Using
nullinstead of0for gap-filled time slots means line charts will visually show gaps, and aggregation functions (avg/min/max) in the legend will correctly skip these empty slots. Good change.
1006-1006:sortedSeriesandlegendAggregationare consistently wired into both chart types.Both bar and line chart render paths use
sortedSeriesfor theseriesprop and passconfig.aggregationaslegendAggregation. The stacking guardstacked && sortedSeries.length > 1on line 1055 is a sensible addition.Also applies to: 1014-1014, 1018-1018, 1041-1041, 1045-1045, 1055-1055
811-821: Chart config colors are now assigned by sorted order.Since
sortedSeriesis used for color assignment viagetSeriesColor(i), the largest series will consistently get the first color. This is a good UX improvement for visual stability.apps/webapp/app/components/primitives/charts/ChartLegendCompound.tsx (4)
10-16:aggregationLabelsusingRecord<AggregationType, string>is a good pattern.This ensures type safety — if a new aggregation type is added to
AggregationType, this record will produce a compile error until it's updated. Clean approach.
72-94: Null handling incurrentTotalis well-implemented.The three-phase approach — collect raw values preserving nulls, filter to non-null, return null when all are null — correctly distinguishes gap-filled points from points with actual zero data. The en-dash (
\u2013) rendering for null is a nice UX touch.
221-246:SimpleTooltipwrapping for truncated legend labels is a good UX improvement.The tooltip shows the full label on hover while the display truncates via CSS
truncate. UsingdisableHoverableContentprevents the tooltip from interfering with legend mouse interactions, andmin-w-0on the button allows the flex child to shrink properly.
313-346:HoveredHiddenItemRowcorrectly handles nullable values.The component properly renders the en-dash for
nullandAnimatedNumberfor real values, consistent with the main legend items.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
matt-aitken
force-pushed
the
metrics-fixes
branch
from
93e0631 to
d24a81b
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (!acquireResult.success) { | ||
| const errorMessage = | ||
| acquireResult.reason === "key_limit" | ||
| ? `You've exceeded your query concurrency of ${orgLimit} for this organization. Please try again later.` | ||
| ? `You've exceeded your query concurrency of ${orgLimit} for this project. Please try again later.` | ||
| : "We're experiencing a lot of queries at the moment. Please try again later."; | ||
| return { success: false, error: new QueryError(errorMessage, { query: options.query }) }; | ||
| } |
There was a problem hiding this comment.
🚩 Shared concurrency key between metric widgets and query page creates contention
The concurrency limiter key was changed from organizationId to projectId at apps/webapp/app/services/queryService.server.ts:141. Both the metric widget path (resources.metric.tsx:111, limit=30 via METRIC_WIDGET_DEFAULT_ORG_CONCURRENCY_LIMIT) and the query page path (limit=3 via DEFAULT_ORG_CONCURRENCY_LIMIT) share the same Redis sorted set keyed by projectId.
The limiter's Lua script at apps/webapp/app/services/redisConcurrencyLimiter.server.ts:126-128 checks ZCARD(keyKey) >= keyLimit. When a dashboard loads ~15 metric widgets simultaneously (each acquiring a slot with limit=30), the sorted set for that project may have 15 entries. A subsequent query page request checks ZCARD >= 3, finds 15 >= 3, and is rejected even though it's a different use case.
This was a pre-existing issue (previously with orgId key and limits 15 vs 3), but this PR widens the gap (30 vs 3) making it more likely that metric widget load blocks the query page for the same project. The mitigation is that query page users could switch to a different project, which is slightly better than the org-level blocking before. Consider using separate key prefixes or a single consistent limit.
(Refers to lines 139-153)
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary